Repository navigation
Conversation
|
This PR currently combines CI changes, test improvements, and a production code fix, while the title and description focus on CI. Could you please split it into three focused PRs?
Please keep regression tests with the production fix they validate. This split will make each change easier to review, validate, and revert independently. If the PRs depend on each other, please document the dependencies and intended merge order. |
@halibobo1205 Thanks for the review. The original intent was to improve the CI setup and disable automatic test retries. While investigating CI failures on this PR, I added test isolation and resource cleanup improvements, along with the Although the I’ll split the changes as suggested:
I’ll update each PR’s title and description and document any dependencies and the intended merge order. The relevant test and production fixes should land before disabling retries. |
04905f4 to
1a1ea8c
Compare
@halibobo1205 To keep the scope of this PR focused, the flaky test issues are being addressed separately in #6974. This PR will remain focused on CI/build configuration: workflow changes, Rocky Linux dependencies, failure diagnostics, and Gradle test-retry configuration. Since this PR removes automatic test retries, the current CI test failures are expected while the flaky tests are still being addressed. Once #6974 resolves those issues, we can rerun the CI for this PR. |
|
@3for Please merge v4.8.3 into this PR and make sure all CI pass without retry. |
43b86de to
92f2588
Compare
@317787106 Done. v4.8.3 has been merged into this PR, and all CI checks pass without retry. |
| java -jar "$JAR" keystore --help | ||
|
|
||
| - name: Upload test diagnostics | ||
| if: failure() |
There was a problem hiding this comment.
[SHOULD] This upload is skipped exactly when it's most needed: a test that hangs. When a job hits timeout-minutes: 60, GitHub cancels it, and failure() is false for a cancelled job. The same applies to L125, L215 and L303, and to L389, whose step-outcome checks see cancelled, not failure, for the step that hung. For example, macOS job 107038663460 (run 35816373930) hit the 60-minute limit and ended with Build = cancelled, job = cancelled.
Options:
- Give the long Gradle steps a step-level
timeout-minutes(e.g. ~45 on Build, less on the RocksDB/coverage steps) so the total stays under the job limit. A step timeout fails the step, sofailure()fires and the logs get uploaded. - Use
if: ${{ failure() || cancelled() }}(and add|| cancelled()on L389). This also uploads for runs cancelled bycancel-in-progress, which is harmless with 7-day retention.
There was a problem hiding this comment.
@halibobo1205 Good catch. Fixed in 430e431. For coverage-base, I intentionally left the two base test steps on the job timeout: they're continue-on-error, so a tolerated step timeout would let a truncated JaCoCo report become the comparison baseline. A hang there cancels the job (skipping the gate), and diagnostics rely on the cancelled() path.
There was a problem hiding this comment.
@halibobo1205 Reverted in 0ecc68b.
The main reason is that frequent PR updates automatically cancel superseded CI runs. These cancellations are expected and do not necessarily indicate build failures.
Skipping diagnostic uploads for cancelled runs reduces redundant artifacts and upload overhead, while diagnostics remain available for non-cancelled failures, including tolerated base test failures.
- Remove Development Tools and unnecessary packages - Install only JDK 8, git-core and zstd with weak dependencies disabled - Use C.utf8 to avoid installing glibc-langpack-en - Resolve and validate JAVA_HOME so Gradle no longer requires which
- Collect tron-test.log, rotated logs and JUnit XML across five PR build jobs - Run artifact uploads even when preceding steps fail - Use distinct artifact names per job and matrix configuration - Retain artifacts for 7 days and warn when no files are found
Remove the stale integration-test-multinode.yml reference to avoid unnecessary API requests when cancelling workflows for closed PRs.
- Save build, RocksDB test and coverage output with tee and plain console mode - Use Bash pipefail to preserve failures when capturing stdout and stderr - Include console logs and HTML test reports in diagnostic artifacts - Preserve existing test retry and base coverage failure policies
Remove the test-retry plugin and retry configuration shared by test and testWithRocksDb so test failures fail the task without retrying.
- Limit diagnostic artifacts to **/logs/tron-test.log and ci-logs/*.log - Upload logs only when a preceding step fails - Include base test failures tolerated by continue-on-error - Keep the existing 7-day retention period
- Set DNF max_parallel_downloads to 10 for Rocky dependency installation - Restore the test-retry plugin and configuration as comments, keeping retries disabled
Set DNF minrate=256k and timeout=30 to abort persistently slow connections and allow fallback to another mirror.
Remove --no-build-cache from Debian and baseline coverage jobs to preserve the cache behavior introduced in tronprotocol#6938.
- Upload diagnostics on failure or cancellation with a 4-minute timeout. - Add step timeouts to reserve time for diagnostic uploads. - Keep Gradle daemon shutdown failures non-fatal. - Retain job-level timeouts for base tests to avoid accepting a partial coverage baseline after a tolerated step timeout. - Preserve outcome checks for tolerated base test failures.
92f2588 to
430e431
Compare
| @@ -1,5 +1,5 @@ | |||
| plugins { | |||
| id "org.gradle.test-retry" version "1.5.9" | |||
| // id "org.gradle.test-retry" version "1.5.9" | |||
There was a problem hiding this comment.
[SHOULD] Please remove test-retry from verification-metadata.xml too.
Remove verification entries for the disabled test-retry plugin and its plugin marker artifact.
- Wrap PR build, test, coverage and Toolkit stages with a shared runner - Collect JVM crash logs, daemon logs, memory samples and OOM counters - Record test classes and methods associated with each test executor - Show failure evidence in Actions annotations and job summaries - Upload stage-specific diagnostics and available JUnit XML on failure - Preserve command exit codes when diagnostic logging fails - Use PR diagnostic scripts for base coverage builds and retain logs for tolerated failures (cherry picked from commit 2f61ae1)
This reverts commit 430e431. Explicitly exclude cancelled runs from all diagnostic upload conditions, while retaining uploads for ordinary failures and tolerated base test failures. Preserve the later Gradle and JVM diagnostic runner integration.
Merge upstream a901f85. Preserve disabled test-retry and cancellation upload guards while accepting the upstream Sonar plugin removal.
5 participants
What does this PR do?
C.utf8, and configureJAVA_HOMEexplicitly.**/logs/tron-test.logandci-logs/*.logonly on failure, with 7-day retention.Why are these changes required?
Unnecessary packages increase CI setup time. Missing console logs make test failures harder to diagnose, while automatic retries can hide intermittent failures. Workflow cancellation should also reflect the workflows that still exist.
This PR has been tested by: